Skip to content

Clean Code for bundles/org.eclipse.core.commands#3258

Merged
laeubi merged 2 commits intomasterfrom
clean-code/bundles/org.eclipse.core.commands
Sep 18, 2025
Merged

Clean Code for bundles/org.eclipse.core.commands#3258
laeubi merged 2 commits intomasterfrom
clean-code/bundles/org.eclipse.core.commands

Conversation

@eclipse-platform-bot
Copy link
Copy Markdown
Contributor

The following cleanups were applied:

  • Add final modifier to private fields
  • Add missing '@Deprecated' annotations
  • Add missing '@Override' annotations
  • Add missing '@Override' annotations to implementations of interface methods
  • Convert control statement bodies to block
  • Make inner classes static where possible
  • Remove trailing white spaces on all lines
  • Remove unnecessary array creation for varargs
  • Remove unnecessary suppress warning tokens
  • Remove unused imports
  • Remove unused private constructors
  • Remove unused private fields
  • Remove unused private methods
  • Remove unused private types
  • Replace deprecated calls with inlined content where possible
  • Use pattern matching for instanceof


// Check if they're the same type.
if (!(object instanceof HandleObject)) {
if (!(object instanceof final HandleObject handle)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjohnstn this looks a bit strange and it seems final is not added to all instanceof checks (where I assume we likely never modify variables used in instanceof checks anyways)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "add final" cleanup was modified to start with a fresh AST and apply previous cleanups. So, if you asked for the pattern instanceof cleanup, the final cleanup will see the new pattern variable that was added and will make it final if conditions are appropriate. In the past, the final cleanup would not have seen the new pattern instanceof expression.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjohnstn so while your explanation seems valid to me, I suspect something is wrong here, because I would not have expected this because we only enabled to make fields as

Add final modifier to private fields

but not for local variables or parameters...

So is my expectation maybe wrong?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, in this case, the declaration of handle that is used to make the pattern instanceof variable is declared final and casts object to HandleObject. The code is doing the right thing. You have a final HandleObject called handle in both cases.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I begin to understand so because previously

final HandleObject handle= (HandleObject) object;

is defined final this is applied to the instanceof as well.. now it seems obvious, thanks for the pointer!

@eclipse-platform-bot
Copy link
Copy Markdown
Contributor Author

eclipse-platform-bot commented Sep 16, 2025

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch
From b72234f50c0fe1bd0ef3563450ac5ec129f4e334 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Thu, 18 Sep 2025 02:35:20 +0000
Subject: [PATCH] Version bump(s) for 4.38 stream


diff --git a/bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF b/bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF
index e9976632db..f3b945668b 100644
--- a/bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.core.commands/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.core.commands
-Bundle-Version: 3.12.400.qualifier
+Bundle-Version: 3.12.500.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.core.commands,
-- 
2.51.0

Further information are available in Common Build Issues - Missing version increments.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 16, 2025

Test Results

 2 904 files  ±0   2 904 suites  ±0   2h 8m 43s ⏱️ - 2m 39s
 8 017 tests ±0   7 771 ✅ ±0  245 💤 ±0  1 ❌ ±0 
23 585 runs  ±0  22 802 ✅ ±0  782 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit cc1be79. ± Comparison against base commit de7ee8c.

♻️ This comment has been updated with latest results.

@eclipse-platform-bot eclipse-platform-bot force-pushed the clean-code/bundles/org.eclipse.core.commands branch 2 times, most recently from d15b102 to 7c6b103 Compare September 17, 2025 09:04
@eclipse-platform-bot eclipse-platform-bot force-pushed the clean-code/bundles/org.eclipse.core.commands branch from a308562 to 0cfd0be Compare September 18, 2025 02:31
@laeubi laeubi merged commit 64daf2d into master Sep 18, 2025
16 of 18 checks passed
@laeubi laeubi deleted the clean-code/bundles/org.eclipse.core.commands branch September 18, 2025 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants